Add opt-in getter call-count tracking for protocol properties#353
Add opt-in getter call-count tracking for protocol properties#353farkasseb wants to merge 4 commits into
Conversation
|
Please separate this PR into one for the bug fix and another. |
098b0f4 to
68ad26c
Compare
Get-only protocol properties render as plain stored properties that record
no reads, so a SwiftyMocky `Verify(mock, .once, .someProperty)` getter check
has no `…GetCallCount` to translate to. This adds opt-in, per-property getter
call-count tracking, parallel to the existing setter `SetCallCount`.
Opt in per property via `@mockable(getter: state = true)` (with an `all`
wildcard and `name = false` opt-out, mirroring `combine:`/`rx:`), or
project-wide via `--enable-getter-history`; explicit annotations win over the
flag. A tracked property becomes a counting computed accessor over a `_name`
backing store, and stays settable so it can still be stubbed:
private(set) var stateGetCallCount = 0
private var _state: T! // or `= <default>` when defaultable
var state: T {
get { stateGetCallCount += 1; return _state }
set { _state = newValue } // + SetCallCount when get-set
}
Scope and guards (protocol mocks only):
- Class mocks (--mock-all) are a no-op (overriding `var` would break definite
init of the backing store).
- Combine/Rx/@Published-intercepted properties and weak/dynamic-modified
properties are excluded by type/modifier; direct Rx subjects stay eligible.
- Init writes the backing store directly (via `backingName`) so neither counter
moves during initialization; an explicit init param sharing a tracked backing
name is de-duplicated against the template-emitted `_name`.
- `UniqueModelGenerator` strips the new `GetCallCount` suffix before the generic
`CallCount` so a re-ingested `stateGetCallCount` keys back to `state`.
Per-property opt-in resolves to an explicit `GetterHistory` enum
(enabled/disabled/unspecified); `.unspecified` defers to the flag. Counters are
plain properties (actor-isolated on actors, unsynchronized under @unchecked
Sendable), matching the existing property `SetCallCount` model.
Adds Tests/TestGetterHistory with @fixture string-diff fixtures (compile-checked
nested mocks) across specific/all/opt-out, get-only/get-set, defaultable/
optional/non-defaultable, static, global-flag, allow-set-call-count, exclusion,
Rx-subject, class-mock no-op, processed-merge, init-collision dedup, existential
`any`/namespaced types, actor, and Sendable, plus runtime tests that read the
compiled fixture mocks to assert counters increment on read and stay 0 after
init. Full suite green (171 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend getter call-count tracking to `{ get async }` / `{ get throws }` /
`{ get async throws }` getters, which render via the handler-backed `.computed`
path (no backing store). `shouldTrackGetter` now also admits `.computed`
getters; the counter is declared alongside the handler and bumped as the first
statement of the getter body — before any `await`/`throw`, so a read that
throws is still counted:
private(set) var valueGetCallCount = 0
var valueHandler: (() async -> Int)?
var value: Int {
get async {
valueGetCallCount += 1
...
}
}
`backingName` still returns the plain `underlyingName` for `.computed` props, so
no `_name` backing is invented and init/dedup are untouched (an init param for
such a property continues to set its handler). When tracking is off the
`.computed` branch is byte-for-byte unchanged.
Adds async/throws/async-throws fixtures plus runtime tests asserting the counter
increments per `await`ed read and that a throwing read is counted. Full suite
green (174 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
68ad26c to
51ae6ab
Compare
|
Rebased on master now that #354 has landed, and re-added the promised getter-on-IUO edge-case fixture as a separate commit: getterHistoryEdgeTypes now includes an Int! property, whose tracked getter renders a valid Int! backing store (previously this produced the Int?! that #354 fixed). While re-testing with Fable 5, 'we' also hardened coverage with three more compiled fixtures: getter: all = false overriding --enable-getter-history (as the README promises), a tracked getter on an associatedtype property, and a test locking that inherited members follow the declaring protocol's annotation (same semantics as history:/rx:/combine:). |
Resolves the computed-var template conflict by combining uber#353's getter call-count declaration with uber#352's attribute prefix. Also converts the getter-history Combine fixture to the dummy Combine types that uber#352 introduced in Tests/Types.swift (extended with Published/DummyPublisher), since the local dummies shadow the real Combine import — this also fixes the fixture's Linux incompatibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
I’m afraid I cannot accept this proposal. |
|
@sidepelican Thank you so much for merging the other PRs and taking the time to review this one as well. Let me try to convince you anyway. First, it'd be fully opt-in via annotation (or the flag), mocks that don't request it are byte-for-byte unchanged, no default-behavior change. Second, a structural point worth naming: mockolo currently supports the setter call-count tracking (...SetCallCount / --allow-set-call-count). So this PR wouldn't add a new pattern, it'd just complete an existing one, the read-side mirror of it. Third, I'd mostly agree with you that the "was this read?" assertions are usually redundant with the outcome but IMHO not always. Three categories the outcome genuinely can't capture:
final class LoginViewModel {
private let interactor: AuthInteracting // var authType / var isBiometricEnabled { get }
func onViewAppeared() {
guard interactor.authType == .biometric else { return } // PIN -> return before touching biometrics
if interactor.isBiometricEnabled { // isBiometricEnabled backed by LAContext / Keychain
coordinator.promptForBiometrics()
}
}
}
---
// SwiftyMocky test for the PIN path:
Given(interactor, .authType(getter: .pin))
// mockolo: interactor.authType = .pin
sut.onViewAppeared()
Verify(interactor, .never, .isBiometricEnabled) // proves we short-circuited
// mockolo: #expect(interactor.isBiometricEnabledGetCallCount == 0)
func configure(_ rows: [Transaction]) {
let formatter = settings.currencyFormatter // built once, reused for every row
cells = rows.map { Cell(amount: formatter.string(from: $0.amount)) }
}
---
// rebuilding a NumberFormatter per row is the expensive part — output is identical either way
Verify(settings, .once, .currencyFormatter)
// mockolo: #expect(settings.currencyFormatterGetCallCount == 1)
func send(_ request: Request, retries: Int) {
for _ in 0...retries {
var attempt = request
attempt.token = session.accessToken // re-read each attempt — token may have refreshed after a 401
if transport.send(attempt).isSuccess { return }
}
}
---
// 1 initial try + 2 retries
Verify(session, 3, .accessToken) // re-read every attempt, not hoisted/cached once
// mockolo: #expect(session.accessTokenGetCallCount == 3)Either way, thanks again to you and Uber for maintaining this project, it's saved me a ton of hand-written mocks, I really love mockolo. |
|
I understand that you prefer writing getters with side effects. |
Summary
Opt-in, per-property getter call-count tracking for protocol mocks — the getter analogue of the existing setter
…SetCallCount. Nothing is tracked unless explicitly opted in, so existing mocks are unchanged and there's no mock bloat. This is especially useful for SwiftyMocky migrations, whereVerify(mock, .once, .someProperty)maps directly onto readingmock.somePropertyGetCallCount.Example
generates:
Use
all = trueto opt every property in,name = falseto opt one back out, and--enable-getter-historyto enable it project-wide (an explicitname = falsestill wins over the flag). Aget setproperty additionally bumps the existing…SetCallCount; async/throwing getters inject the counter into the getter body.Scope
Protocol mocks only (a no-op for
--mock-allclass mocks). Combine/Rx/@Published-intercepted publishers andweak/dynamic-modified properties are never tracked, since they can't be backed by a counting computed accessor.Test plan
Tests/TestGetterHistoryfixtures (compiled@Fixturemocks, so conformance is enforced, not just string-matched) covering: specific /all/ opt-out, get-only / get-set, defaultable / optional / non-defaultable, IUO / existential / namespaced / associatedtype types, static, global flag (includingall = falseoverriding it),--allow-set-call-count, Combine/Rx/@Published/weak/dynamic exclusions, direct Rx subjects, inherited-member annotation semantics, class-mock no-op, processed-mock merge, init-collision dedup, actor, Sendable, and async/throwing getters — plus runtime tests asserting counters increment on read and stay 0 after init.The IUO type-rendering fix that the compiled fixtures originally surfaced was split out into #354 per review. It has since landed, and the getter-on-IUO edge-case test has been re-added here, along with rebasing on master.
🤖 Generated with Claude Code